Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Appropriately wait for worker child process when shutting down via SIGTERM #1437

Closed
wants to merge 1 commit into from

Conversation

shayonj
Copy link
Contributor

@shayonj shayonj commented Oct 11, 2017

This way when a SIGTERM is issued to a puma cluster (master process)
the shutting down of workers/child processes successfully completes.

Before this, it meant the master process will wait for every child process, not just
puma specific child process. Which meant even though the workers were to
successfully terminate, the server never exits as intended.

Issue: #1386

@shayonj shayonj changed the title Only wait for worker child process when shutting down via SIGTERM Appropriately wait for worker child process when shutting down via SIGTERM Oct 11, 2017
@shayonj shayonj force-pushed the s/exit-waitpid branch 3 times, most recently from 5108f53 to c62ccf5 Compare October 11, 2017 21:54
…GTERM

This way when a SIGTERM is issued to a puma cluster (master process)
the shutting down of workers/child processes successfully completes.

Before this, it meant the master process will wait for every child process, not just
puma specific child process. Which meant even though the workers were to
successfully terminate, the server never exits as intended.
MSP-Greg added a commit to MSP-Greg/puma that referenced this pull request Nov 19, 2017
nateberkopec pushed a commit that referenced this pull request Nov 20, 2017
@shayonj
Copy link
Contributor Author

shayonj commented Nov 20, 2017

@nateberkopec @MSP-Greg is this good to close (and so the mentioned issue)? Looks the changes were rolled into a single PR. :)

@MSP-Greg
Copy link
Member

@shayonj

Please accept my apology, as I should have mentioned you in the PR. It was kind of a 'if all three are rolled into one, this should work' thing...

@nateberkopec
Copy link
Member

Yes - thanks you for opening, @shayonj.

@shayonj
Copy link
Contributor Author

shayonj commented Nov 20, 2017

not at all @MSP-Greg. Thanks all!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants